-
Notifications
You must be signed in to change notification settings - Fork 57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Try/refactor editor UI #563
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking great! I've been through each option and noted some small suggestions. The only issues I noticed were when testing the "clone" workflow.
Improve the navigation options to make them easier to understand
This is a fantastic improvement - it's much better than the current state and feels easier to iterate on where needed.
Add ability for user to control what aspects of the theme are saved
Another great improvement on the current workflow ✨
I noticed when clicking the "Save Changes" button that any unsaved changes are lost on reload - there is a prompt that changes will be lost but it is shown after the "Save Changes" action has already been performed. Should we prompt the user to save changes via the editor first? Or make it more clear somehow that the editor has not yet been saved?
Clarifies Creation of a Theme Variation
Nice, this works well!
Moved Metadata editor to a Modal
This is great, it allows us to include the long list of available tags as we had on the wp-admin page and gives us more room to expand these options.
Simplifies Blank Theme Creation
Looks good and works well in my testing.
Make the "Clone" workflow into a wizard workflow
When I tried cloning the current theme as a regular theme, I saw the "The theme you are currently using is not compatible with the Site Editor." error. The theme created only included a theme.json file and no other files. I was using Programme at the time, so perhaps it was related to the base theme. I also tried with TT3 as the base theme, and I didn't see an error but the new theme wasn't created, and rendered as a blank page in the editor:
Co-authored-by: Sarah Norris <1645628+mikachan@users.noreply.github.com>
Co-authored-by: Sarah Norris <1645628+mikachan@users.noreply.github.com>
I think this would be a good add. My 'druthers is to not hold up this work with that, but I agree it would be helpful. I'll open a separate issue for that. |
… saving template changes
That makes sense, thank you for creating the new issue 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great!
When cloning a theme, the name of the previous app is replaced everywhere, including places where I think it's either not needed, or where we need to take extra steps:
- When exporting the theme without applying translations or asset localization, the name of existing theme assets are changed in the pattern code, but not on the filesystem:
- Changelog entries in the readme are being replaced with the new given name for the theme; we should consider either cleaning the slate, or not replacing the name.
This isn't new behavior and likely has happened forever. We're just (kind of blindly) doing a string-replace of old/new slugs. So if one of the theme assets (in the case you raised, an image) has the name of the slug in the filename we replace that (and shouldn't). The fix is to have smarter slug replacement but... I don't know how we would do that. I think we should open an issue with that theme specifically and see what we can do about it in the near future.
Also existing behavior, and I agree should be changed. I would opt for a clean slate (it's a new theme after all). Since it's existing behavior I would also prefer to open an issue and address that in the near future (not this branch). |
I agree that avoiding replacing the Changelog entries is optional, but not changing the asset's names in the filesystem sound like a bug to me; we're adding broken links and unused assets to the theme files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested again, and it works well, keeping in mind the issues created from this.
Changes the UI in the Editor to simplify and enhance in the following ways:
Improve the navigation options to make them easier to understand:
Fixes #538
Gives users easier to understand options, grouping the "Do things to this theme" options and "Make a new theme" options and giving the creation workflow a wizard instead of a bunch of confusing options.
Add ability for user to control what aspects of the theme are saved:
Fixes #535
Instead of just a 'Save' button a user is presented with a panel with the option to save/modify aspects if the theme including:
theme.json
file and removed from the database)Clarifies Creation of a Theme Variation
Fixes #559
While it was always possible to name a style variation it was confusing and unclear. This new workflow makes it clear how to do that.
Moved Metadata editor to a Modal
The Metadata editor was already cramped and is expected to grow. This change moves the form to a modal clearing the way for more and better enhancements.
Simplifies Blank Theme Creation
Prompting for only a single field (a Name) creating a blank theme has been simplified. (The new MetaData modal is available to edit the new theme further and limited metadata options are still available on the form).
Make the "Clone" workflow into a wizard workflow
This is the bit that fixes #538
A user is prompted to determine what kind of theme they are making (a regular theme or a child theme) and then how they want it created (created and activated on the server or just exported as a zip file).